Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for REST server extensions #1101

Merged
merged 1 commit into from
May 7, 2019

Conversation

ggriffiths
Copy link
Contributor

Signed-off-by: Grant Griffiths [email protected]

What this PR does / why we need it:

  • Makes the gRPC SDK Rest Gateway server extendable.

Which issue(s) this PR fixes (optional)
Closes #1100

Special notes for your reviewer:

@ggriffiths ggriffiths requested a review from lpabon May 2, 2019 22:58
@@ -155,6 +168,11 @@ func (s *sdkRestGateway) restServerSetupHandlers() (http.Handler, error) {
}
}

// Register extra services provided
if err := s.registerServerExtensions(gmux, conn); err != nil {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we do it this way, the caller will have to setup a function with a for loop to register their functions. During registering of the gRPC function, the single function the caller registers does not need a loop.

I think it would be cleaner to accept an array of handlers that take (ctx, gmux, conn) that way they can be appended to the handlers on line 164 and automatically registered.

What do you think?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The works too - that's one of the ways I considered setting it up for gRPC. I was thinking that the way we set them up should be similar for both gRPC and REST, as but as long as we document the behavior it should be fine.

I can change it to accept a slice of handlers

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@lpabon I've changed it to just be a slice of handlers, and they're started the same way as the core handlers are created.

@ggriffiths ggriffiths force-pushed the rest_server_extensions branch from 98fe9fe to 93e40ac Compare May 6, 2019 18:54
@lpabon lpabon merged commit 274a1bb into libopenstorage:master May 7, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

SDK REST Gateway server cannot be extended
2 participants